Skip to content

Non-darwin test discovery with Swift Build #8722

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jun 24, 2025
Merged

Non-darwin test discovery with Swift Build #8722

merged 1 commit into from
Jun 24, 2025

Conversation

owenv
Copy link
Contributor

@owenv owenv commented May 25, 2025

Still needs a fair amount of work and cleanup, I'll look at breaking off some parts that can land independently. Depends on the larger patch in swiftlang/swift-build#499

@owenv owenv marked this pull request as draft May 25, 2025 02:02
owenv
owenv commented May 25, 2025
View reviewed changes
owenv
owenv commented May 25, 2025
View reviewed changes
owenv
owenv commented May 25, 2025
View reviewed changes
@owenv
Copy link
Contributor Author

owenv commented May 25, 2025

Locally, I can pass a decent chunk of TestDiscoveryTests on linux. macOS will need a bit of work to skip building the test runner executables, and I expect Windows will need a few tweaks

@swift-ci test

jakepetroules
jakepetroules reviewed May 25, 2025
View reviewed changes
@owenv owenv force-pushed the owenv/test-discovery branch from a18182a to 10bc51d Compare May 25, 2025 20:30
@owenv
Copy link
Contributor Author

owenv commented May 25, 2025

swiftlang/swift-build#499
@swift-ci test

@owenv owenv marked this pull request as ready for review May 25, 2025 20:38
@owenv owenv changed the title WIP: Non-darwin test discovery with Swift Build Non-darwin test discovery with Swift Build May 25, 2025
@owenv owenv force-pushed the owenv/test-discovery branch from 10bc51d to 401a93f Compare June 4, 2025 18:20
@owenv
Copy link
Contributor Author

owenv commented Jun 4, 2025

@swift-ci test

@owenv owenv force-pushed the owenv/test-discovery branch from 401a93f to 3757526 Compare June 9, 2025 16:10
@owenv
Copy link
Contributor Author

owenv commented Jun 9, 2025

@swift-ci test

bkhouri
bkhouri reviewed Jun 10, 2025
View reviewed changes
Copy link
Contributor

@bkhouri bkhouri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for converting the TestDiscoverTests from XCTest to Swift Testing. I have a couple questions/comments, but nothing blocking.

@owenv owenv force-pushed the owenv/test-discovery branch from 633a864 to 88ccf4a Compare June 10, 2025 18:13
@owenv
Copy link
Contributor Author

owenv commented Jun 10, 2025

@swift-ci test

@owenv owenv force-pushed the owenv/test-discovery branch from 88ccf4a to 23a8d43 Compare June 11, 2025 16:45
@owenv
Copy link
Contributor Author

owenv commented Jun 11, 2025

@swift-ci test

@owenv owenv force-pushed the owenv/test-discovery branch from 23a8d43 to 61c3ec0 Compare June 11, 2025 18:33
@owenv
Copy link
Contributor Author

owenv commented Jun 20, 2025

@swift-ci test

@owenv owenv force-pushed the owenv/test-discovery branch from 9cfbd82 to e63dc87 Compare June 20, 2025 19:27
@owenv
Copy link
Contributor Author

owenv commented Jun 20, 2025

@swift-ci test

@owenv owenv force-pushed the owenv/test-discovery branch from e63dc87 to 70dcd09 Compare June 20, 2025 20:51
bkhouri
bkhouri reviewed Jun 21, 2025
View reviewed changes
bkhouri
bkhouri reviewed Jun 21, 2025
View reviewed changes
bkhouri
bkhouri requested changes Jun 21, 2025
View reviewed changes
Copy link
Contributor

@bkhouri bkhouri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only reviewed the tests changes. Although the overcharge is good, we should ensure to run the TestDicoveryTests against SwiftBuild on Windows and use the withKnownIssue Swift Testing API to essentially mark the test as "expected fail". This will give us signal when the underlying Windows issue is fix, thus prompting the removal of the withKnownIssue as opposed to having to remember to augment the test.

jakepetroules
jakepetroules reviewed Jun 21, 2025
View reviewed changes
@owenv owenv force-pushed the owenv/test-discovery branch 3 times, most recently from 1b4de2a to e73ba32 Compare June 22, 2025 18:13
@owenv
Copy link
Contributor Author

owenv commented Jun 23, 2025

@swift-ci test

@owenv
Copy link
Contributor Author

owenv commented Jun 23, 2025

@swift-ci test Windows

@owenv
Copy link
Contributor Author

owenv commented Jun 23, 2025

Windows: 'TestCommandSwiftBuildTests.testListWithSkipBuildAndNoBuildArtifacts' failed (14.613 seconds)

@owenv owenv force-pushed the owenv/test-discovery branch from e73ba32 to a180198 Compare June 23, 2025 21:14
@owenv
Copy link
Contributor Author

owenv commented Jun 23, 2025

@swift-ci test

@owenv owenv force-pushed the owenv/test-discovery branch from a180198 to 96ab7a9 Compare June 23, 2025 21:56
@owenv
Copy link
Contributor Author

owenv commented Jun 23, 2025

@swift-ci test

@owenv owenv requested a review from bkhouri June 23, 2025 22:06
bkhouri
bkhouri approved these changes Jun 23, 2025
View reviewed changes
Tests/FunctionalTests/TestDiscoveryTests.swift
struct TestDiscoveryTests {
static var buildSystems: [BuildSystemProvider.Kind] = [BuildSystemProvider.Kind.native, .swiftbuild]

@Test(arguments: buildSystems)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (non-blocking, if-pr-update): using arguments: SupportedBuildSystemOnAllPlatforms does essentially the same thing :)

Tests/FunctionalTests/TestDiscoveryTests.swift
}

func testNonStandardName() async throws {
@Test(.bug("https://github.com/swiftlang/swift-build/issues/13"), arguments: [BuildSystemProvider.Kind.native])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: out of curiosity, why aren't we executing this test against the SwiftBuild build system?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some design questions I need to sort out related to response file quoting on non-Darwin platforms and it's not 100% clear to me marking this as a known issue is the right thing to do yet

@bkhouri
Copy link
Contributor

bkhouri commented Jun 23, 2025

@swift-ci test windows

@owenv
Copy link
Contributor Author

owenv commented Jun 24, 2025

@swift-ci smoke test macOS

@owenv
Copy link
Contributor Author

owenv commented Jun 24, 2025

@swift-ci test macOS

@owenv
Copy link
Contributor Author

owenv commented Jun 24, 2025

macOS checkout issue
@swift-ci test macOS

@owenv owenv merged commit 3db835d into swiftlang:main Jun 24, 2025
6 checks passed
marcprux pushed a commit to swift-everywhere/swift-package-manager that referenced this pull request Jul 7, 2025
@owenv @marcprux
Non-darwin test discovery with Swift Build (swiftlang#8722)
d095775 
Still needs a fair amount of work and cleanup, I'll look at breaking off
some parts that can land independently. Depends on the larger patch in
swiftlang/swift-build#499
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jakepetroules jakepetroules jakepetroules approved these changes

@bkhouri bkhouri bkhouri approved these changes

@dschaefer2 dschaefer2 dschaefer2 approved these changes

@shawnhyam shawnhyam Awaiting requested review from shawnhyam shawnhyam is a code owner

@bripeticca bripeticca Awaiting requested review from bripeticca bripeticca is a code owner

@plemarquand plemarquand Awaiting requested review from plemarquand plemarquand is a code owner

@cmcgee1024 cmcgee1024 Awaiting requested review from cmcgee1024 cmcgee1024 is a code owner

@daveyc123 daveyc123 Awaiting requested review from daveyc123 daveyc123 is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@owenv @bkhouri @jakepetroules @dschaefer2